-
Notifications
You must be signed in to change notification settings - Fork 80
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
qt-build-utils: Add an option to use CMake instead of QMake #1156
base: main
Are you sure you want to change the base?
Conversation
0fb6d0b
to
bd7a8bd
Compare
bd7a8bd
to
9f7238c
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1156 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 71 71
Lines 11967 11967
=========================================
Hits 11967 11967 ☔ View full report in Codecov by Sentry. |
9f7238c
to
66c1972
Compare
This is needed for scenarios where the CMake packaging for Qt is better than what QMake can handle, e.g. split Qt installations where the modules live in different directories. A new optional feature is added to cxx-qt-build to prefer CMake for most operations related to linking/including Qt modules. QMake is still the default. Fixes KDAB#1153.
66c1972
to
1d29d33
Compare
} | ||
} | ||
|
||
return paths |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this makes the code after it unreachable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is really interesting, i wonder if the approach here could be.
- Make the qmake mode a feature
- Have the fallbacks all by
unimplemented!()
- Default qt-build-utils to enable the qmake feature
- Potentially change things to be trait driven for implementations here? (see below)
- Add code for a CMake feature
- Potentially change the default in the future easily
Whether as part of the refactor @LeonMatthesKDAB we change the code of qt-build-utils to be trait driven, so instead of every function having a two cfg blocks and a unimplemented instead there is a trait that is implemented by either CMake/QMake implementations ? Then the question is whether the feature is even needed and both are always built, but maybe the feature chooses the "default" implementation and you can still manually force one of them...
let Some(target) = package.target(format!("Qt6::{}", tool_name)) else { | ||
return Err(()); | ||
}; | ||
return target.location.ok_or(()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
This is needed for scenarios where the CMake packaging for Qt is better than what QMake can handle, e.g. split Qt installations where the modules live in different directories.
A new optional feature is added to cxx-qt-build to prefer CMake for most operations related to linking/including Qt modules. QMake is still the default.
Fixes #1153.